-
Notifications
You must be signed in to change notification settings - Fork 3.4k
Improving aria-label warnings #344
Conversation
|
@marcysutton - as part of #340 [Refactor to use ngAria], we need to move these service methods to Utils and not use a custom $aria material service (since it conflicts). This also impacts #314. |
|
Ok. I figured the service would just be renamed. |
|
If we want to use
|
|
@ThomasBurleson any input on the timing issue mentioned above? |
|
@marcysutton - can I voip with you tomorrow AM after 11:00 AM CST? |
e582b55 to
94a233c
Compare
src/components/slider/slider.spec.js
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adding TestUtil.mockRaf here causes the existing tests to fail. But without it, $materialAria.expect does not run.
94a233c to
828cb4e
Compare
src/services/aria/aria.js
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ajoslin @ThomasBurleson This causes an issue when there is more than one component of the same type on a page: $materialAria.expect only executes for the last instance. Need to figure out a way to delay expectAttribute from running but still execute for every component that needs it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah. right.
Just do:
function expectAttribute() {
$$rAF(function() {
//expectAttribute logic
});
}
bca0431 to
bad82be
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Passing false for copyElementText will accommodate components requiring a warning for aria-label if the developer forgot, such as material-slider. Passing true will try to copy the element's text (or use the optional defaultValue, like in the case of material-dialog). I'm open to renaming this parameter to something that sounds more boolean (or refactoring).
0615d2b to
2ea9e3f
Compare
42c80f7 to
4c0c50e
Compare
2ea9e3f to
063e61d
Compare
063e61d to
a8cef8f
Compare
docs/app/js/app.js
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We include ngAria as a dependency of ngMaterial itself now, so this is extraneous.
a8cef8f to
e6363c8
Compare
|
Merged via 3368c93. |
When a button, checkbox, radio button or slider is missing alternative text, warnings are logged to the developer telling them about the problem and directing them to the live node requiring a fix.
Closes #342